Skip to content

Format library #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Jan 4, 2024
Merged

Format library #18

merged 8 commits into from
Jan 4, 2024

Conversation

sfe-SparkFro
Copy link
Collaborator

Will resolve #9

I have not yet changed any names, just ran the files through a formatter. @gigapod, you mentioned wanting to change some names away from snake case. I'm also not sold on all the names I chose. Do you have any suggestions?

@sfe-SparkFro
Run library source through formatter
c0214de 
Clang formatter in VS Code set to Microsoft style
@sfe-SparkFro
Run examples through formatter
14c52f3 
Clang formatter in VS Code set to Microsoft style
@sfe-SparkFro
Copy link
Collaborator Author

Also, are the file names and directory structure okay? If we're going to change those, would probably be best to do that as part of this PR. And any other house-cleaning things like that, really.

gigapod
gigapod reviewed Dec 15, 2023
View reviewed changes
Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. The only thing to tweak is the class names - see comment I left.

@PaulZC
Copy link
Contributor

PaulZC commented Dec 15, 2023

u-blox is preferred.
ublox 2nd choice.
(They prefer without capitalisation....)

@sfe-SparkFro
Change sprintf to snprintf
6a7ee7f
@sfe-SparkFro
Don't use calloc for fixed length arrays
363ec5c 
Many command and response arrays are fixed length, so no need to use calloc. This helps prevent the heap from getting fragmented.
@sfe-SparkFro
Change strstr to strnstr
7dc48df 
Fixes #12
@sfe-SparkFro
Run formatter again
0995691 
clang with Microsoft style
@sfe-SparkFro
Rename classes
9259452
@sfe-SparkFro
Copy link
Collaborator Author

@gigapod @PaulZC Changes pushed. Also decided to address #12 in this PR, and I removed a bunch of calls to ubx_cell_calloc_char() in favor of putting arrays on the stack; should help with preventing the heap from getting fragmented, and give a minor performance improvement.

Please take a look and let me know what you think! If all is okay, I'd like to get the first release out this week if possible. There's still a couple open issues, but it should at least be totally useable.

@sfe-SparkFro
Copy link
Collaborator Author

@PaulZC @gigapod Last call for input before I merge this early next week. Thanks!

gigapod
gigapod approved these changes Jan 2, 2024
View reviewed changes
Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - just one very minor comment/nit.

@sfe-SparkFro sfe-SparkFro merged commit 13ffd73 into v1.0.0 Jan 4, 2024
@sfe-SparkFro sfe-SparkFro deleted the format_library branch January 4, 2024 19:53
@sfe-SparkFro sfe-SparkFro mentioned this pull request Jan 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gigapod gigapod gigapod approved these changes

@PaulZC PaulZC Awaiting requested review from PaulZC

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfe-SparkFro @PaulZC @gigapod